Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tileengine rotated #402

Merged
merged 8 commits into from
Jun 21, 2024
Merged

Tileengine rotated #402

merged 8 commits into from
Jun 21, 2024

Conversation

sdeseille
Copy link
Contributor

Hello @straker

The last update about flipping vertically or horizontally don't cover all cases. Indeed, if a tile is rotated 90° clockwise or anticlockwise, Tiled use bits 30.

I implemented that feature. I hope to have carefuly follow your coding style and do the thing the right way.

I'm open to your feedback about my PR.

regards

@sdeseille

@straker
Copy link
Owner

straker commented May 9, 2024

Thanks for the PR. I took a look and one thing I noticed is that tiles can be both flipped and rotated, but the code right now only handles one or the other.

@sdeseille
Copy link
Contributor Author

Hello @straker Thanks for your feedback. If you're ok I will work on that point.
regards
@sdeseille

@sdeseille
Copy link
Contributor Author

Hello @straker , would you help me to build a test and add it to your repo ? I studied how Tiled works and the way it save its maps and effectively you can't save a tile with the information that it is flipped and rotated. With a test it will be more simple to explain the cases.
@sdeseille

@straker
Copy link
Owner

straker commented May 26, 2024

Sure. Here's the test file I was playing with. It takes the map pack Tileset from Kenney and just rotates the snowman to each position, then does the same with the image flipped both directions.

image
{ 
  "compressionlevel":-1,
 "height":4,
 "infinite":false,
 "layers":[
        {
         "data":[106, 2684354666, 3221225578, 1610612842,
            2147483754, 3758096490, 1073741930, 536871018,
            1073741930, 536871018, 2147483754, 3758096490,
            3221225578, 1610612842, 106, 2684354666],
         "height":4,
         "id":4,
         "name":"decoration",
         "opacity":1,
         "type":"tilelayer",
         "visible":true,
         "width":4,
         "x":0,
         "y":0
        }],
 "nextlayerid":5,
 "nextobjectid":1,
 "orientation":"orthogonal",
 "properties":[
        {
         "name":"sx",
         "type":"string",
         "value":"10"
        }, 
        {
         "name":"sy",
         "type":"string",
         "value":"10"
        }],
 "renderorder":"right-down",
 "tiledversion":"1.10.2",
 "tileheight":64,
 "tilesets":[
        {
         "columns":17,
         "firstgid":1,
         "image":"mapPack_tilesheet.png",
         "imageheight":768,
         "imagewidth":1088,
         "margin":0,
         "name":"mapPack_tilesheet",
         "spacing":0,
         "tilecount":204,
         "tileheight":64,
         "tilewidth":64
        }],
 "tilewidth":64,
 "type":"map",
 "version":"1.10",
 "width":4
}

@sdeseille
Copy link
Contributor Author

Hi @straker, I created a sample on Codepen with your example. Thanks a lot, it permitted me to observe that my implementation is not fully compatible with Tiled. I am continuing to work on it.

@sdeseille
Copy link
Contributor Author

Hello @straker I finally solved all cases to flip, rotate and flip-rotate the tile to be compatible with the tiled save format.
Can you take a look and give me some feedback?
regards
@sdeseille

@straker
Copy link
Owner

straker commented Jun 19, 2024

Thanks for the updated code. Tested it and it works. The one thing it's lacking is some tests. I can go ahead and merge this and add them or if you'd like you can add some tests for diagonally rotated tiles following the same test I did for rotated tiles.

@sdeseille
Copy link
Contributor Author

Hello @straker, thanks for your review.
I will follow the links you shared to add a new test for diagonally rotated tiles.

I'll keep you inform when i'm ready.

Regards

@sdeseille

@sdeseille
Copy link
Contributor Author

Hello @straker, I understood how to add unit tests to cover the new features.
Take a look and give me your feedback.

regards
@sdeseille

@straker
Copy link
Owner

straker commented Jun 21, 2024

Fantastic! Everything looks good. Thanks again for adding this.

@straker straker merged commit 388afb5 into straker:main Jun 21, 2024
9 checks passed
@sdeseille sdeseille deleted the tileengine-rotated branch June 22, 2024 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants